Assertion failure: mOwner == ExpectedOwnerForChild(aFrame) (Missed some frame in the hierarchy?), at /src/layout/base/ServoRestyleManager.cpp:154 #0 mozilla::ServoRestyleState::ChangesHandledFor(nsIFrame const&) const /src/layout/base/ServoRestyleManager.cpp:153:3 #1 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:807:35 #2 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #3 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #4 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #5 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #6 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #7 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #8 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #9 mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:952:32 #10 mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:1141:28 #11 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /src/layout/base/PresShell.cpp:4147:41 #12 nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1926:18 #13 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /src/layout/base/nsRefreshDriver.cpp:307:7 #14 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:329:5 #15 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:770:5 #16 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:683:35 #17 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:584:9 #18 mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /src/layout/ipc/VsyncChild.cpp:67:16 #19 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:155:20 #20 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1755:28 #21 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2119:25 #22 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2049:17 #23 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1895:5 #24 mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1928:15 #25 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1037:14 #26 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:512:10 #27 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21 #28 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10 #29 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3 #30 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27 #31 XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:877:22 #32 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:269:9 #33 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10 #34 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3 #35 XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:703:34 #36 content_process_main(mozilla::Bootstrap*, int, char**) /src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30 #37 main /src/browser/app/nsBrowserApp.cpp:280:18 #38 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291 #39 _start (/home/user/workspace/browsers/m-c-1508348667-asan-debug/firefox+0x41ebe4)

INFO: Last good revision: 11de870775c1c9b395dade9d780afb4183c43fb4 INFO: First bad revision: 15fc474cbc387b9fc8326f18e968651f242587f2 INFO: Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=11de870775c1c9b395dade9d780afb4183c43fb4&tochange=15fc474cbc387b9fc8326f18e968651f242587f2

FWIW, that patch added the assertion, because it fixed a bug like this... That doesn't mean I can't take a look to it of course, will do tomorrow morning too.

This one is fun, this is display: contents messing the insertion point parent with XBL / Shadow DOM.

There's some repetition in the new code: GetPrimaryFrame ... GetDisplayContentsStyleFor ... GetFlattenedTreeParent occurs twice. Could we write it something like this instead: nsIContent* content = aContent; nsIFrame* frame; while (!(frame = content->GetPrimaryFrame())) { if (!GetDisplayContentsStyleFor(content)) { return nullptr; } content = content->GetFlattenedTreeParent(); if (!content) { return nullptr; } } ...

Sure thing.

Comment on attachment 8920546 [details] Bug 1410226: Properly compute the insertion point for a display: contents child in an XBL binding. https://reviewboard.mozilla.org/r/191554/#review196816 Thanks!

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f8703e5ad323 Properly compute the insertion point for a display: contents child in an XBL binding. r=mats

Emilio, should we uplift this fix to Beta 57?

Backed out for failing reftests layout/reftests/forms/legend/shadow-dom.html and layout/reftests/css-display/display-contents-generated-content-2.html: https://hg.mozilla.org/integration/autoland/rev/98f4e20b334813ee19fa5de2cc1117d0dc9ae70c Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f8703e5ad323b63ee97e75b0dffb960d3cc9ca54&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry For display-contents-generated-content-2.html the last light green span is shown/missing, for layout/reftests/forms/legend/shadow-dom.html the heading for box 6.

Argh, I ran display-contents-shadow-dom, but not those, thanks Aryx. No, we don't need to uplift this, we've shipped this bug for a long time, it's just a stylo assertion that happens to catch it.

Oh, I'm stupid, this is of course because of the rewrite mats suggested, and I messed up. I kept: if (frame->GetContent() != aContent) But now it shouldn't really be checking aContent. Sigh.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2245ad9f35b3c0c51b6865b7af68b53d7a65841, with the fix, just in case... Will land tomorrow morning since the fix is just: - if (frame->GetContent() != aContent) { + if (frame->GetContent() != content) { Leaving ni? for that.

Alternatively, just use aContent throughout? We don't really need the original value, and it's a bit of a foot-gun to have both aContent/content variables like this, so when the function is short enough I think it's OK to clobber the param, although I usually try to avoid that. Either way is fine with me, but if you keep the local var then please name it something more distinct from aContent to avoid it being misread. (my bad of suggesting "content" in the first place)

Agreed... Can't think of a better name, so I'll just reuse aContent.

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/cc74db25e470 Properly compute the insertion point for a display: contents child in an XBL binding. r=mats